Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Remove duplicate code and reduce GET calls #29

Open
wants to merge 12 commits into
base: main
Choose a base branch
from
Open

Remove duplicate code and reduce GET calls #29

wants to merge 12 commits into from

Conversation

trivikr
Copy link
Member

@trivikr trivikr commented Nov 13, 2024 •
edited
Loading

A subset of #28

Notes

Splits the code into three folders:

  • dist: Existing folder for GitHub Action
  • cache: Cached files for schedule and vulnerability list
    • This cache is latest at the time of writing, so two GET calls will not be made. We can set up a GitHub action to update the cache in future.
  • src: Source code
    • index.js: Existing root file.
    • ascii.js: existing file for colors removed.
    • is-node-vulnerable.js: Existing file with some duplicate code removed.
    • is-node-eol.js: Renamed eol-versions.js, but it uses the cache if JSON is not provided.
    • store.js: Data store to be shared between different files.

Testing

Before

Uses cached version for end-of-life v16.20.2

$ DEBUG=1 node ./index.js

██████   █████  ███    ██  ██████  ███████ ███████
██   ██ ██   ██ ████   ██ ██       ██      ██   ██
██   ██ ███████ ██ ██  ██ ██   ███ █████   ███████
██   ██ ██   ██ ██  ██ ██ ██    ██ ██      ██   ██
██████  ██   ██ ██   ████  ██████  ███████ ██   ██


v16.20.2 is end-of-life. There are high chances of being vulnerable. Please upgrade it.

Downloads from upstream for vulnerable v20.0.0

$ DEBUG=1 node ./index.js
Creating local core.json
Creating local core.json

██████   █████  ███    ██  ██████  ███████ ███████
██   ██ ██   ██ ████   ██ ██       ██      ██   ██
██   ██ ███████ ██ ██  ██ ██   ███ █████   ███████
██   ██ ██   ██ ██  ██ ██ ██    ██ ██      ██   ██
██████  ██   ██ ██   ████  ██████  ███████ ██   ██
...

Downloads from upstream for vulnerable v20.18.0

$ DEBUG=1 node ./index.js
Loading local ETag for 'security'
Loading local ETag for 'schedule'
Creating local core.json
Creating local core.json

 █████  ██      ██           ██████   ██████   ██████  ██████         ██
██   ██ ██      ██          ██       ██    ██ ██    ██ ██   ██     ██  ██
███████ ██      ██          ██   ███ ██    ██ ██    ██ ██   ██         ██
██   ██ ██      ██          ██    ██ ██    ██ ██    ██ ██   ██     ██  ██
██   ██ ███████ ███████      ██████   ██████   ██████  ██████         ██

After

Uses cached version for end-of-life v16.20.2

$ DEBUG=1 node ./src/index.js

██████   █████  ███    ██  ██████  ███████ ███████
██   ██ ██   ██ ████   ██ ██       ██      ██   ██
██   ██ ███████ ██ ██  ██ ██   ███ █████   ███████
██   ██ ██   ██ ██  ██ ██ ██    ██ ██      ██   ██
██████  ██   ██ ██   ████  ██████  ███████ ██   ██

v16.20.2 is end-of-life. There are high chances of being vulnerable. Please upgrade it.

Uses cached version for vulnerable v20.0.0

$ DEBUG=1 node ./src/index.js
Loading local ETag for 'security'
Loading local ETag for 'schedule'
No updates from upstream. Getting a cached version: /Users/trivikram/workspace/is-my-node-vulnerable/cache/schedule.json
No updates from upstream. Getting a cached version: /Users/trivikram/workspace/is-my-node-vulnerable/cache/security.json


██████   █████  ███    ██  ██████  ███████ ███████
██   ██ ██   ██ ████   ██ ██       ██      ██   ██
██   ██ ███████ ██ ██  ██ ██   ███ █████   ███████
██   ██ ██   ██ ██  ██ ██ ██    ██ ██      ██   ██
██████  ██   ██ ██   ████  ██████  ███████ ██   ██

...

Uses cached version for non-vulnerable v20.18.0

$ DEBUG=1 node ./src/index.js
Loading local ETag for 'security'
Loading local ETag for 'schedule'
No updates from upstream. Getting a cached version: /Users/trivikram/workspace/is-my-node-vulnerable/cache/schedule.json
No updates from upstream. Getting a cached version: /Users/trivikram/workspace/is-my-node-vulnerable/cache/security.json


 █████  ██      ██           ██████   ██████   ██████  ██████         ██
██   ██ ██      ██          ██       ██    ██ ██    ██ ██   ██     ██  ██
███████ ██      ██          ██   ███ ██    ██ ██    ██ ██   ██         ██
██   ██ ██      ██          ██    ██ ██    ██ ██    ██ ██   ██     ██  ██
██   ██ ███████ ███████      ██████   ██████   ██████  ██████         ██

@trivikr trivikr mentioned this pull request Nov 13, 2024
Closed
@trivikr

This comment was marked as outdated.

Sign in to view
@trivikr trivikr mentioned this pull request Nov 13, 2024
Open
@trivikr trivikr marked this pull request as ready for review November 19, 2024 06:22
@trivikr
Copy link
Member Author

trivikr commented Nov 19, 2024

The PR is ready for review.

I used path.resolve() so that vercel ncc can understand the location of cache directory.

@trivikr trivikr marked this pull request as draft November 19, 2024 06:42
@trivikr

This comment was marked as outdated.

Sign in to view
@trivikr trivikr marked this pull request as ready for review November 19, 2024 07:56
@trivikr
Copy link
Member Author

trivikr commented Nov 19, 2024

The PR is ready for review.

I used typeof __NCC__ to use path.resolve()

@trivikr
Copy link
Member Author

trivikr commented Nov 19, 2024

I'm not sure why dist/index.js reduces from 1.3 MB in main to 160.7 kB in my workspace even in main branch.
I've reverted changes in dist/index.js, so that rest of the code can be reviewed.

@trivikr trivikr force-pushed the reduce-duplicate-code branch from 44892c2 to d3a9423 Compare November 19, 2024 08:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
1 participant
@trivikr